Skip to content

feat(cryptdConnectionString): makes mongocryptd uri configurable#2049

Merged
rosemaryy merged 4 commits intomongodb:nextfrom
rosemaryy:NODE-2012
Jul 3, 2019
Merged

feat(cryptdConnectionString): makes mongocryptd uri configurable#2049
rosemaryy merged 4 commits intomongodb:nextfrom
rosemaryy:NODE-2012

Conversation

@rosemaryy
Copy link
Contributor

Fixes NODE-2012

Description

Previously, cryptd's connectionString was not configurable. Now, the user has the option of setting connectionString by passing it in as an option for MongoClient.

What changed?
cryptdConnectionString is now a valid option for MongoClient

@rosemaryy rosemaryy requested review from daprahamian, kvwalker, mbroadst and spal1 and removed request for daprahamian and mbroadst July 1, 2019 21:01
Copy link
Member

@mbroadst mbroadst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rosemaryy, two comments:

  • I don't think this change is performance related, I think you might want to call this a feat
  • The options for client-side field-level encryption are all represented in the autoEncryption option. We should adhere to the spec when it comes to setting options related to mongocryptd

@rosemaryy rosemaryy changed the title perf(cryptdConnectionString): makes mongocryptd uri configurable feat(cryptdConnectionString): makes mongocryptd uri configurable Jul 2, 2019
Copy link
Member

@mbroadst mbroadst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rosemaryy rosemaryy merged commit 597262c into mongodb:next Jul 3, 2019
@rosemaryy rosemaryy deleted the NODE-2012 branch July 3, 2019 17:18
daprahamian pushed a commit that referenced this pull request Aug 13, 2019
* perf(cryptdConnectionString): make mongocryptd uri configurable by creating the property `autoEncryption.extraOptions.mongocryptURI`

Fixes NODE-2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants